src/block/carousel/frontend-carousel.js (1)

680-683: 💤 Low value

Modulo arithmetic prevents the crash but has an edge case with the duplicate lastClone.

The modulo successfully clamps all clone indices back into [1, slideEls.length], which prevents the TypeError mentioned in issue #3695. However, there's a subtle semantic issue:

When slidesToShow equals slideEls.length, the init code creates a duplicate clone of the last slide (lastClone in addition to the regular clone). For example, with 3 slides:

• Combined array indices 1-3: original slides 1-3
• Indices 4-6: clones of slides 1-3
• Index 7: duplicate lastClone of slide 3

The modulo ((7-1) % 3) + 1 = 1 maps this to slide 1, when semantically it represents slide 3. This could cause the wrong dot to be highlighted or other minor UI inconsistencies if the user scrolls to a position where the lastClone is nearest.

That said, the fix achieves its primary objective of preventing crashes. The old logic (subtracting length once) would still leave index 7 out of range (7 - 3 = 4), so this is a clear improvement. The semantic edge case is unlikely to cause serious issues in practice, since the carousel's swap logic typically keeps users away from the lastClone position during normal scrolling.

Alternative approach for perfect semantic correctness

If the lastClone edge case becomes problematic, consider tracking which source slide each clone represents:

 if ( this.infiniteScroll ) {
-  // Clamp clone indexes back to the original slide range.
-  slide = ( ( slide - 1 ) % this.slideEls.length ) + 1
+  // Map clones back to their source slide
+  if ( slide > this.slideEls.length ) {
+    const cloneIndex = slide - this.slideEls.length - 1
+    // clones[0..slidesToShow-1] map to slides 1..slidesToShow
+    // clones[slidesToShow] (lastClone) maps to slideEls.length
+    slide = cloneIndex < this.slidesToShow ? cloneIndex + 1 : this.slideEls.length
+  }
 }

However, the current modulo approach is simpler and adequate for preventing the crash.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/block/carousel/frontend-carousel.js` around lines 680 - 683, The modulo
fix prevents out-of-range indices but mishandles the special duplicate lastClone
when slidesToShow === this.slideEls.length; update the logic in the block where
this.infiniteScroll is handled (the code that currently does slide = ((slide -
1) % this.slideEls.length) + 1) to special-case the duplicate lastClone: detect
when the index corresponds to the extra lastClone (e.g., by checking
slidesToShow === this.slideEls.length and that the computed/initial slide index
is the extra clone position or by inspecting the DOM node/class/clone metadata
for "lastClone") and map that case to this.slideEls.length instead of applying
the modulo; otherwise keep the modulo mapping.